Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX Clear filter from url state #1580

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Sep 11, 2023

Issue #1573

Requires the following PRs merged first:

Installer run with all 3x PRs together:

Have run yarn upgrade webpack because js build was randomly failing - relates to #1582

@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch 2 times, most recently from 513f2fc to 88357e2 Compare September 12, 2023 01:04
@emteknetnz emteknetnz marked this pull request as ready for review September 12, 2023 01:05
@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch from 88357e2 to d407f02 Compare September 12, 2023 01:11
Copy link
Contributor

@sabina-talipova sabina-talipova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Steve, it seems to me that now it doesn’t work quite the way it worked before. Because when search for smth and we go to the next level, for example, we click on one of the list elements and there we go to one of the nested lists and when we go back, we lose all the previous settings (filters, sorting and pagination).

Screen.Recording.2023-09-13.at.1.56.49.PM.mov

@@ -23,3 +28,15 @@ Feature: Search in GridField
And I click "Walmart" in the "#Form_EditForm" element
Then I should see "Walmart"
And I should see "Walmart" in the ".breadcrumbs-wrapper" element

@sboyd
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@sboyd

@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch from d407f02 to 594db46 Compare September 13, 2023 21:47
@emteknetnz
Copy link
Member Author

I think this is an existing behaviour, I can replicated that on 4.13 without this pull-request

The gridState is lost when you view the individual employee "Troy"

// aggressive in removing their state. This limitation is because I couldn't find an
// easy way to extract the gridfield number from the gridfield
if (key.match(new RegExp(`^gridState\\-${gridFieldName}\\-[0-9]$`))) {
console.log('Removing ' + key);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.log('Removing ' + key);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

tests/behat/features/gridfield-search.feature Show resolved Hide resolved
tests/behat/features/gridfield-search.feature Show resolved Hide resolved
client/src/legacy/GridField.js Show resolved Hide resolved
@sabina-talipova
Copy link
Contributor

There are also some failed tests. Please, have a look.
Thank you.

@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch 4 times, most recently from fa1b4ae to 0d7985b Compare September 14, 2023 23:07
@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch from 0d7985b to 9ba03f3 Compare September 15, 2023 03:13
Copy link
Contributor

@sabina-talipova sabina-talipova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just small changes.

tests/behat/features/gridfield-search.feature Show resolved Hide resolved
client/src/legacy/GridField.js Show resolved Hide resolved
@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch 3 times, most recently from b9c7088 to 7ed648b Compare September 18, 2023 05:57
@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch from 7ed648b to 0b7d69b Compare September 19, 2023 04:00
@emteknetnz emteknetnz force-pushed the pulls/1.13/clear-gridfield-search branch from 0b7d69b to f9a151f Compare September 19, 2023 04:33
Copy link
Contributor

@sabina-talipova sabina-talipova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Works nice in local environment.

@sabina-talipova sabina-talipova merged commit 2fea4a6 into silverstripe:1.13 Sep 19, 2023
12 checks passed
@sabina-talipova sabina-talipova deleted the pulls/1.13/clear-gridfield-search branch September 19, 2023 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants